Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

FYST-1778 MD wants more bank info in financial resolution on xml #5566

Merged
merged 8 commits into from
Feb 12, 2025

Conversation

arinchoi03
Copy link
Contributor

@arinchoi03 arinchoi03 commented Feb 11, 2025

Link to pivotal/JIRA issue

Is PM acceptance required? (delete one)

  • Yes - don't merge until JIRA issue is accepted!

Reminder: merge main into this branch and get green tests before merging to main

What was done?

  • Filled in bank details for MD intakes inside the AuthenticationHeader
    • only if direct_deposit for either refund OR owed situation but only for MD
  • Also refactored tests in FinancialTransaction (I was there accidentally, thought I was adding stuff there but turned out i was supposed to be adding to AuthenticationHeader 😓 )

How to test?

  • Describe the testing approach taken to verify the changes, including:
    • Unit tests
    • Test data used

Screenshots (for visual changes)

  • MD Refund
Screenshot 2025-02-11 at 3 31 17 PM
  • MD Owed
Screenshot 2025-02-11 at 3 33 55 PM
  • ID Refund
Screenshot 2025-02-11 at 3 37 34 PM
  • ID Owed
Screenshot 2025-02-11 at 3 39 13 PM

Copy link

Heroku app: https://gyr-review-app-5566-1031c436f04a.herokuapp.com/
View logs: heroku logs --app gyr-review-app-5566 (optionally add --tail)

…en (routing num, acct num, signature should exist at this point)
…ut in FinancialResolution unless direct_deposit payment indicated
described_class.build(
submission, validate: false, kwargs: kwargs
).document.to_xml)
}
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

refactored tests in FinancialTransaction (I was here accidentally, thought I was adding stuff here but turned out i was supposed to be adding to AuthenticationHeader 😓)

xml.FirstInput do
xml.RoutingTransitNum sanitize_for_xml(@intake.routing_number) if @intake.routing_number.present?
xml.DepositorAccountNum sanitize_for_xml(@intake.account_number) if @intake.account_number.present?
xml.InputTimestamp @intake.primary_esigned_at.in_time_zone(StateFile::StateInformationService.timezone("md")).strftime("%FT%T%:z") if @intake.primary_esigned_at.present?
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

found the strftime format needed to satisfy xsd schema https://apidock.com/ruby/DateTime/strftime

@arinchoi03 arinchoi03 changed the title Fyst 1778 md update financial resolution on xml FYST-1778 MD wants more bank info in financial resolution on xml Feb 12, 2025
@@ -7,6 +7,13 @@ def document
build_xml_doc("AuthenticationHeader") do |xml|
xml.FilingLicenseTypeCd "O"
xml.FinancialResolution do
if @intake.has_banking_information_in_financial_resolution? && @intake.payment_or_deposit_type == "direct_deposit"
Copy link
Contributor Author

@arinchoi03 arinchoi03 Feb 12, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thought of checking whether routing/account/signed at exists here (b/c all those are required if FirstInput is present -- but felt like direct_deposit check was enough to ensure those things would be present (routing/acct no) and we should have the esigned_at at this point...but if we want to check all of them I won't be mad about it

Copy link
Contributor

@embarnard embarnard left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good! A few minor comments

context "refund" do
let(:intake) { create(:state_file_md_refund_intake, primary_esigned_at: DateTime.new(2025, 2, 15, 12).in_time_zone(StateFile::StateInformationService.timezone("md"))) }

it "fills out the FirstInput banking information" do
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should there be another context here specifying payment_or_deposit_type is direct_deposit?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ooh :state_file_md_refund_intake sets payment_or_deposit_type to direct_deposit but I can explicitly set it

context "owed" do
let(:intake) { create(:state_file_md_owed_intake, primary_esigned_at: DateTime.new(2025, 2, 15, 12).in_time_zone(StateFile::StateInformationService.timezone("md"))) }

it "fills out the FirstInput banking information" do
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same here

@arinchoi03 arinchoi03 merged commit cdad4df into main Feb 12, 2025
7 checks passed
@arinchoi03 arinchoi03 deleted the FYST-1778-md-update-financial-resolution-on-xml branch February 12, 2025 21:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants